Skip to content

Use should_build_on_full for bid parent block hash#5310

Merged
jtraglia merged 6 commits into
ethereum:masterfrom
nflaig:bid-use-should-build-on-full
Jun 1, 2026
Merged

Use should_build_on_full for bid parent block hash#5310
jtraglia merged 6 commits into
ethereum:masterfrom
nflaig:bid-use-should-build-on-full

Conversation

@nflaig

@nflaig nflaig commented May 29, 2026

Copy link
Copy Markdown
Member

seems like this was missed in #5186

@github-actions github-actions Bot added the gloas label May 29, 2026

@jtraglia jtraglia left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch 👍

@jtraglia jtraglia marked this pull request as draft May 29, 2026 17:55
@nflaig nflaig marked this pull request as ready for review May 29, 2026 19:11
Comment thread specs/gloas/validator.md Outdated
Comment thread specs/gloas/validator.md Outdated
Comment thread specs/gloas/validator.md

#### Constructing the `BeaconBlockBody`

Let `head = get_head(store)` be the parent block the proposer is building on,

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we might wanna use get_proposer_head here cc @michaelsproul

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jtraglia we can't use get_proposer_head because it returns Root and not ForkChoiceNode, but we can make that change separate from this PR which makes it just a 1-line change later

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good. I'm going to merge this now & let you make the follow up PR.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let you make the follow up PR.

you mean the ForkChoiceNode refactor? no idea what's the effort for this or how to do it correctly as I haven't look into that but seems same refactoring we wanna do to FCR cc @mkalinin

in any case, there is no rush to do this, our implementation uses get_proposer_head already and I suspect all other clients that implement proposer boost reorg do too

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm okay. So we should merge this PR & deal with the refactor after the release?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see how it matters for the release, we can even use get_proposer_head already, I don't think the linter catches this?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Handled in this PR.

@jtraglia jtraglia merged commit 027fc72 into ethereum:master Jun 1, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants